Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ffaa1ba64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| type SystemInstall struct { | ||
| RunAs RunAs | ||
| Account SystemAccount |
There was a problem hiding this comment.
Existing deployments become unreadable after upgrading: every previously encoded blueprint-resolved-v1 document contains a RunAs JSON member (even when empty), while DecodeResolvedDocumentV1 uses DisallowUnknownFields and the schema identifier remains unchanged. Consequently, staging, install, runtime, and recovery operations fail while decoding any pre-change state-v1; preserve the old wire name or add an explicit state migration/schema transition before renaming this field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR unifies application-container sandbox policy into a single validated ApplicationSandboxPlanV1 used by both persistent workload rendering (Compose) and transient command execution, while also renaming the blueprint system-install identity field from run_as to account and updating documentation + tests accordingly.
Changes:
- Introduce
ApplicationSandboxPlanV1and enforce its validation across runtime planning, Docker input rendering, and transient command specs. - Refactor
DockerExecutionPlanto embed sandbox policy underSandbox(replacingRuntimeUser/TemporaryHome) and update call sites/tests. - Rename blueprint schema node
environment.install.system.run_as→environment.install.system.account, updating decode/resolve paths and docs.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| website/docs/blueprint-structure.md | Updates blueprint example to system.account. |
| internal/dockerdeploy/state_v1_test_helpers_test.go | Adds helper to build canonical sandbox plans for tests. |
| internal/dockerdeploy/runtime_plan.go | Validates sandbox plan during runtime plan construction. |
| internal/dockerdeploy/runtime_plan_test.go | Updates runtime plan tests to provide sandbox plans where needed. |
| internal/dockerdeploy/provider_install_run.go | Switches install account preparation to SystemAccount. |
| internal/dockerdeploy/provider_install_run_test.go | Updates provider install tests for SystemAccount and sandbox presence. |
| internal/dockerdeploy/provider_install_plan_test.go | Adjusts assertions to read sandbox runtime identity. |
| internal/dockerdeploy/provider_install_account.go | Renames run-as handling to system-account handling and updates errors. |
| internal/dockerdeploy/provider_install_account_test.go | Updates account tests to use SystemAccount. |
| internal/dockerdeploy/provider_build_run_test.go | Updates build tests to read runtime identity from sandbox. |
| internal/dockerdeploy/provider_build_prepare_test.go | Ensures fixtures include a sandbox plan. |
| internal/dockerdeploy/provider_build_completion_test.go | Updates drift detection tests with the new plan shape. |
| internal/dockerdeploy/private_workload_environment_test.go | Updates rendering tests to supply sandbox policy. |
| internal/dockerdeploy/private_workload_environment_integration_test.go | Updates integration tests to supply sandbox policy. |
| internal/dockerdeploy/live_run_container_test.go | Updates transient execution fixture to supply sandbox policy. |
| internal/dockerdeploy/execution_render.go | Validates sandbox and consumes Sandbox fields when rendering Compose inputs. |
| internal/dockerdeploy/execution_render_test.go | Updates Compose rendering test to use sandbox plan. |
| internal/dockerdeploy/execution_plan.go | Refactors DockerExecutionPlan to carry Sandbox and validates it during planning. |
| internal/dockerdeploy/execution_plan_test.go | Updates runtime user scope policy tests for SystemAccount and sandbox identity. |
| internal/dockerdeploy/doctor.go | Updates doctor to inspect System.Account. |
| internal/dockerdeploy/doctor_test.go | Updates doctor tests for SystemAccount. |
| internal/dockerdeploy/current_shell_run_test.go | Adjusts checks for Sandbox.TemporaryHome. |
| internal/dockerdeploy/current_runtime_plan.go | Updates installed runtime reconstruction to use System.Account. |
| internal/dockerdeploy/current_runtime_plan_test.go | Updates assertions to use sandbox runtime identity and account terminology. |
| internal/dockerdeploy/current_runtime_files_test.go | Updates file-plan fixture to provide sandbox plan. |
| internal/dockerdeploy/current_build_verify_test.go | Updates verification fixtures to include sandbox. |
| internal/dockerdeploy/current_build_reuse_test.go | Updates reuse drift tests with the new plan shape. |
| internal/dockerdeploy/current_app_command_run.go | Uses sandbox runtime identity when preparing output. |
| internal/dockerdeploy/current_app_command_run_test.go | Updates assertions to compare sandbox runtime identity. |
| internal/dockerdeploy/command_execution.go | Validates sandbox and consumes sandbox policy when building transient docker commands. |
| internal/dockerdeploy/command_execution_test.go | Updates transient command tests to supply sandbox plans. |
| internal/dockerdeploy/command_execution_integration_test.go | Updates integration tests to supply sandbox plans. |
| internal/dockerdeploy/application_sandbox_plan.go | Adds canonical sandbox plan type + validation. |
| internal/dockerdeploy/application_sandbox_plan_test.go | Adds coverage ensuring renderer parity and validation behavior. |
| internal/blueprint/syntax.go | Renames install syntax node from run_as to account. |
| internal/blueprint/syntax_test.go | Adds coverage for new terminology and legacy field rejection. |
| internal/blueprint/resolve.go | Maps syntax System.Account into resolved model. |
| internal/blueprint/model.go | Renames model types/fields to SystemAccount / Account. |
| docs/CROSS_PLATFORM_INSTALL_LOCATIONS.md | Updates references to system install account naming. |
| docs/CONTROLLED_SESSION_DESIGN.md | Updates design text to reflect account semantics. |
| docs/BLUEPRINT_ENVIRONMENT_MODEL.md | Updates schema documentation and explanatory text for system.account. |
| docs/BLUEPRINT_ENVIRONMENT_IMPLEMENTATION_PLAN.md | Updates implementation plan references to system.account. |
| .changes/unreleased/+linux-user-scope-install.yaml | Updates changelog fragment to reference install.system.account. |
| .changes/unreleased/+canonical-application-sandbox.yaml | Adds changelog fragment for canonical sandbox + rename. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := ValidateApplicationSandboxPlanV1(plan.Sandbox); err != nil { | ||
| return CommandSpec{}, fmt.Errorf("render application sandbox: %w", err) | ||
| } |
11d4210 to
a3f5e0f
Compare
bd53fee to
ff9d3ee
Compare
ded453d to
11b32d0
Compare
Route persistent workloads and transient application commands through one validated sandbox plan. Rename the system-install identity field to account and cover renderer parity and schema migration.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Route persistent workloads and transient application commands through one validated sandbox plan. Rename the system-install identity field to account and cover renderer parity and schema migration.
Stack created with Sapling. Best reviewed with ReviewStack.